Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove timezone migration #2115

Merged
merged 2 commits into from
Mar 26, 2021
Merged

Remove timezone migration #2115

merged 2 commits into from
Mar 26, 2021

Conversation

NikkiWines
Copy link
Contributor

Details

Removes a migration that was causing issues in the following PR: #2040

Fixed Issues

Fixes https://github.com//pull/2040#issuecomment-808354538

Tests/QA

  1. Log into E.cash
  2. Open the JS console navigate to Application, select Local Storage, and search for myPersonalDetails
  3. Manually modify your timezone in your myPersonalDetails so that it's a string like so
    OLD
"timezone": {
  "automatic": true,
  "selected": "America/Los_Angeles"
}

NEW

"timezone": "America/Los_Angeles"
  1. Refresh the page and confirm you don't see any errors in the JS console and the page loads as anticipated.

Tested On

  • Web
  • Mobile Web
  • Desktop
  • iOS
  • Android

Screenshots

N/A no UI changes

@NikkiWines NikkiWines self-assigned this Mar 26, 2021
@NikkiWines NikkiWines marked this pull request as ready for review March 26, 2021 21:07
@NikkiWines NikkiWines requested a review from a team as a code owner March 26, 2021 21:07
@botify botify requested review from nickmurray47 and removed request for a team March 26, 2021 21:07
roryabraham
roryabraham approved these changes Mar 26, 2021
View reviewed changes
Copy link
Contributor

@roryabraham roryabraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, LGTM. This PR is essentially just a revert of https://github.com/Expensify/Expensify.cash/pull/2040/files

@roryabraham roryabraham merged commit f579946 into master Mar 26, 2021
@roryabraham roryabraham deleted the nikki-undo-timezonemigration branch March 26, 2021 21:24
@OSBotify OSBotify mentioned this pull request Mar 26, 2021
Closed
85 tasks
@NikkiWines
Copy link
Contributor Author

For some reason I am unable to record my screen at the moment but I can confirm that this works on staging

@NikkiWines
Copy link
Contributor Author

Got screen recording working 🎉

Screen.Recording.2021-03-26.at.2.41.51.PM.mov

@isagoico
Copy link

Nice! Need me to test it on my side too?

@NikkiWines
Copy link
Contributor Author

Can't hurt 😅 I'd rather be safe than sorry here

@isagoico
Copy link

Oh no 😟
image

app-6abeeedf7b640e20646e.bundle.js:57 Uncaught TypeError: Cannot read property 'login' of null
    at Zs (app-6abeeedf7b640e20646e.bundle.js:85)
    at n.value (app-6abeeedf7b640e20646e.bundle.js:85)
    at ve.onKeyDown (app-6abeeedf7b640e20646e.bundle.js:1)
    at Object.s (app-6abeeedf7b640e20646e.bundle.js:57)
    at f (app-6abeeedf7b640e20646e.bundle.js:57)
    at app-6abeeedf7b640e20646e.bundle.js:57
    at b (app-6abeeedf7b640e20646e.bundle.js:57)
    at at (app-6abeeedf7b640e20646e.bundle.js:57)
    at ot (app-6abeeedf7b640e20646e.bundle.js:57)
    at st (app-6abeeedf7b640e20646e.bundle.js:57)

@roryabraham
Copy link
Contributor

@isagoico While that's not good, it looks to me like you're using the production website

@isagoico
Copy link

Yep I think the production "set up password" link is broken. I also updated on the staging results for this one here #2040 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@roryabraham roryabraham roryabraham approved these changes

@nickmurray47 nickmurray47 Awaiting requested review from nickmurray47

Assignees

@NikkiWines NikkiWines

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@NikkiWines @isagoico @roryabraham